Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Schema for Expressions #718

Open
wants to merge 47 commits into
base: main
Choose a base branch
from
Open

Schema for Expressions #718

wants to merge 47 commits into from

Conversation

bkeepers
Copy link
Collaborator

@bkeepers bkeepers commented Mar 30, 2023

This introduces a schema based on JSON Schema (draft-07 since it is the latest draft supported by Ruby libraries) for validating expressions (#692) and the arguments to expression functions. It also moves all of the tests into JSON file format so we they can be used when we add support for other clients.

Update: The schemas live in a separate repository, which will also be published as an npm module that will be consumed by flippercloud.io. I tried moving them into this repository, but there were some challenges with using npm modules in a subdirectory.

Punting on this until later:

  • Validate expression before saving, e.g. Flipper.enable_expression :feature, expression

Base automatically changed from learn-the-rules to main July 17, 2023 14:55
- Adds `class Function extends Expression`
- `class Constant extends Expresson`
* commit 'd06da06ecee15d66043dbf563996f7ba4f92bb09': (55 commits)
  Remove kwarg for Strict handler, use arg or block
  Define default Adapter#name
  Update Changelog.md
  Explose handler
  Example for strict adapter
  Spec for proc handler
  Add get_multi, remove name param
  Add Strict adapter to ensure feature exists on get
  Allow head requests to api
  Use base_class to determine flipper_id for ActiveRecord actors
  Bump actions/checkout from 3 to 4
  Ignore asdf version manager file
  Prepare for 1.0.0 release
  Ignore asdf version manager file
  Update links from jnunemaker => flippercloud
  Update links from jnunemaker => flippercloud
  Bump supercharge/mongodb-github-action from 1.9.0 to 1.10.0
  Removed `bool`, `actors`, `time`, `actor`, `percentage_of_actors`, and `percentage_of_time` methods on `Flipper` and `Flipper::DSL`
  Bump version for prerelease
  Update changelog
  ...
* origin/main:
  Make examples and ci metrix exclusions in line with each other
  Don't run 3.2 on rails 5 examples
  Remove telemetry junk that shouldn't have been committed
  Add rails 7.1 and ruby 3.2 to examples and 7.1 to ci
'lib/flipper/expression/schema.rb',
'lib/flipper/version.rb',
] + Dir['node_modules/@flippercloud.io/expressions/schemas/*.json']
gem.name = 'flipper-expressions-schema'
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be flipper-expression-schemas? The ruby class is Flipper::Expression::Schema to avoid conflicts with functions declared in Flipper::Expressions::*, but maybe those should be renamed to Flipper::Function::*. The JS package is @flippercloud.io/expressions

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you just wondering if it should be plural? I think matching to ruby is fine even if singular.

@bkeepers bkeepers marked this pull request as ready for review October 13, 2023 16:35
* origin/main:
  Add changelog note for human readable names
  Use human action name if present
  Make sure actor_names_source can be updated
  Add actor_names_source to configuration
Copy link
Collaborator

@jnunemaker jnunemaker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand this at a high level. I'm not sure what it will feel like moving forward but the best way is to merge and find out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants